GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs (1)

213-219: ApplyPauseAwareProcessingMode() 被重复调用

OnShow 在第 216 行调用 ApplyPauseAwareProcessingMode()，随后第 218 行调用 OnResume()，而 OnResume 内部（第 189 行）也会调用 ApplyPauseAwareProcessingMode()。这导致同一方法在 OnShow 流程中被执行两次。

虽然 ProcessMode 赋值是幂等的，不会导致功能问题，但冗余调用可以移除以保持代码简洁。

♻️ 建议移除 OnShow 中的冗余调用

     public virtual void OnShow()
     {
         _page?.OnShow();
-        ApplyPauseAwareProcessingMode();
         Owner.Show();
         OnResume();
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Godot/UI/CanvasItemUiPageBehaviorBase.cs` around lines 213 - 219,
在 OnShow 方法中移除对 ApplyPauseAwareProcessingMode() 的冗余调用；保留 _page?.OnShow(),
Owner.Show() 和随后调用的 OnResume()（OnResume 已会调用
ApplyPauseAwareProcessingMode()），因此只需删除 OnShow 中直接调用
ApplyPauseAwareProcessingMode() 即可，确保行为不变且避免重复执行。

GFramework.Game.Abstractions/UI/UiInteractionProfile.cs (2)

71-89: Modal 与 Topmost 配置完全相同，存在代码重复。

可合并 switch 分支以减少重复：

♻️ 消除重复的建议

 public static UiInteractionProfile CreateDefault(UiLayer layer)
 {
     return layer switch
     {
-        UiLayer.Modal => new UiInteractionProfile
-        {
-            CapturedActions = UiInputActionMask.Cancel,
-            BlocksWorldPointerInput = true,
-            BlocksWorldActionInput = true
-        },
-        UiLayer.Topmost => new UiInteractionProfile
+        UiLayer.Modal or UiLayer.Topmost => new UiInteractionProfile
         {
             CapturedActions = UiInputActionMask.Cancel,
             BlocksWorldPointerInput = true,
             BlocksWorldActionInput = true
         },
         _ => Default
     };
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Abstractions/UI/UiInteractionProfile.cs` around lines 71 -
89, The CreateDefault method in UiInteractionProfile duplicates the same
initialization for UiLayer.Modal and UiLayer.Topmost; consolidate these branches
into a single case to remove repetition by matching both UiLayer.Modal and
UiLayer.Topmost to the same new UiInteractionProfile (preserving
CapturedActions, BlocksWorldPointerInput and BlocksWorldActionInput) and keep
the default fallback to Default; update the switch in CreateDefault accordingly.

---

9-90: Abstractions 项目包含运行时实现类。

根据编码规范，Abstractions 项目应仅包含接口和契约定义，不应包含运行时实现逻辑。UiInteractionProfile 是一个包含工厂方法 (CreateDefault) 和业务逻辑 (Captures) 的 sealed class，违反了此规范。

建议将此类移动到 GFramework.Game 项目中，或将其重构为纯数据传输对象，将工厂逻辑移至专门的工厂类。

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Abstractions/UI/UiInteractionProfile.cs` around lines 9 - 90,
UiInteractionProfile contains runtime logic (sealed class, CreateDefault factory
and Captures method) which violates the Abstractions-only rule; fix by removing
runtime behavior from the Abstractions assembly: either move the entire
UiInteractionProfile type (including CreateDefault and Captures) into the
GFramework.Game runtime project, or convert UiInteractionProfile into a pure
data DTO (e.g., immutable record with properties Default, CapturedActions,
BlocksWorldPointerInput, BlocksWorldActionInput, PauseMode, PauseGroup,
ContinueProcessingWhenPaused, PauseReason) and relocate the CreateDefault
factory and Captures logic into a runtime factory/class (e.g.,
UiInteractionProfileFactory or UiInteractionProfileExtensions) in
GFramework.Game; ensure all call sites referencing
UiInteractionProfile.CreateDefault or UiInteractionProfile.Captures are updated
to use the new factory/extension or the moved type.

GFramework.Game/UI/UiRouterBase.cs (2)

478-489: 方法名与返回值语义存在歧义。

TryHandleUiAction 的命名暗示返回值表示"是否成功处理了动作"，但实际返回值表示"是否有页面捕获了该动作"（即使 owner.TryHandleUiAction 返回 false，方法仍返回 true）。

这可能导致调用方误解：认为返回 true 意味着动作已被成功处理。建议：

1. 重命名为 TryDispatchUiAction 或 TryCaptureUiAction 更准确反映语义；
2. 或返回更丰富的结果类型区分"已捕获但未处理"与"已捕获且已处理"。

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/UI/UiRouterBase.cs` around lines 478 - 489, The method
TryHandleUiAction currently returns true whenever a page captures the action
even if owner.TryHandleUiAction(action) returned false, causing semantic
confusion; update this by either renaming TryHandleUiAction to
TryDispatchUiAction or TryCaptureUiAction to reflect "capture" semantics, or
change the signature to return a richer result (e.g. an enum or a tuple) that
distinguishes CapturedAndHandled vs CapturedButNotHandled vs NotCaptured; adjust
callers of TryHandleUiAction accordingly and update references to
GetUiActionOwner, owner.TryHandleUiAction, and the Log.Debug call to reflect the
new name or return type so callers can correctly interpret whether the action
was actually handled.

---

838-850: 实例 ID 排序依赖固定宽度格式，存在潜在风险。

当前使用 StringComparer.Ordinal 对实例 ID 进行排序来实现"最近显示优先"。这依赖于 GenerateInstanceId() 生成的固定宽度格式 ui_{id:D6}。

若 _instanceCounter 超过 999999，生成的 ID 如 "ui_1000000" 与 "ui_0999999" 比较时，字符串排序结果将不正确（"0" > "1" 在字符比较中不成立，但 "ui_1000000" 字典序实际小于 "ui_0999999" 的第5字符比较）。

考虑使用数值比较替代字符串比较，或在生成 ID 时使用更大的填充宽度：

♻️ 可选：提取数值进行比较

 private IEnumerable<IUiPageBehavior> EnumerateVisibleLayerPages(UiLayer layer)
 {
     if (!_layers.TryGetValue(layer, out var layerDict))
         yield break;

     foreach (var page in layerDict
-                 .OrderByDescending(static pair => pair.Key, StringComparer.Ordinal)
+                 .OrderByDescending(static pair => ExtractInstanceNumber(pair.Key))
                  .Select(static pair => pair.Value)
                  .Where(static page => page.IsAlive && page.IsVisible))
     {
         yield return page;
     }
 }
+
+private static int ExtractInstanceNumber(string instanceId)
+{
+    // instanceId format: "ui_000001"
+    return int.TryParse(instanceId.AsSpan(3), out var num) ? num : 0;
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game/UI/UiRouterBase.cs` around lines 838 - 850, The sorting for
recent-visible pages in EnumerateVisibleLayerPages relies on string ordering of
instance IDs (generated by GenerateInstanceId as "ui_{id:D6}") which breaks once
_instanceCounter exceeds the fixed width; change the sort to numeric ordering by
extracting the numeric suffix from the key (or, better, store the numeric
counter alongside the page and sort by that). Concretely: in
EnumerateVisibleLayerPages replace OrderByDescending(static pair => pair.Key,
StringComparer.Ordinal) with OrderByDescending using the parsed integer suffix
(e.g., parse after "ui_" or use int.TryParse on the appropriate substring) or
sort by the page's stored instanceCounter field if available, ensuring stable
and correct recent-first ordering even when _instanceCounter grows beyond
999999.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (1)

163-168: 测试路由器未调用 OnInit() 初始化生命周期。

CreateRouter() 仅绑定根节点，未调用 OnInit()，导致 _factory 和 _pauseStackManager 未初始化。当前测试因避开了使用 _factory 的代码路径而能通过，但若未来测试扩展或路由器实现变更，可能产生脆弱性。

建议考虑在 TestUiRouter 中显式暴露初始化方法，或模拟必要依赖。

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 163 - 168,
CreateRouter currently constructs TestUiRouter and only calls BindRoot, leaving
lifecycle initialization undone so OnInit isn't invoked and fields like _factory
and _pauseStackManager remain uninitialized; fix by invoking the router
initialization after binding the root (call the router's initialization method
or OnInit) or modify TestUiRouter to expose an explicit Init/Initialize method
that sets up _factory and _pauseStackManager (or provide mocks for those
dependencies) before returning from CreateRouter so all lifecycle state is ready
for tests that exercise those paths.

GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (2)

33-39: 断言过度绑定到“同一实例”实现细节

这里用 Is.SameAs 会把测试绑定到对象复用策略；若后续改为返回“等价但不同实例”的配置对象，会出现误报。建议断言语义一致性而不是引用相等。

建议修改

         Assert.Multiple(() =>
         {
             Assert.That(modal.CapturedActions, Is.EqualTo(UiInputActionMask.Cancel));
             Assert.That(modal.BlocksWorldPointerInput, Is.True);
             Assert.That(modal.BlocksWorldActionInput, Is.True);
-            Assert.That(topmost, Is.SameAs(modal));
+            Assert.That(topmost.CapturedActions, Is.EqualTo(UiInputActionMask.Cancel));
+            Assert.That(topmost.BlocksWorldPointerInput, Is.True);
+            Assert.That(topmost.BlocksWorldActionInput, Is.True);
         });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 33 - 39,
The test currently asserts reference equality with Assert.That(topmost,
Is.SameAs(modal)) which ties the test to instance reuse; instead assert semantic
equivalence of the topmost and modal configuration (for example compare a stable
identity or key property or use structural equality) inside the same
Assert.Multiple block. Locate the Assert.Multiple in UiRouterInteractionTests
(the modal and topmost locals) and replace the Is.SameAs assertion with an
assertion that checks a semantic property (e.g., Assert.That(topmost.Id,
Is.EqualTo(modal.Id)) or Assert.That(topmost, Is.EqualTo(modal)) assuming Equals
is implemented) so the test verifies behavior rather than object identity.

---

173-179: 建议降低对私有字段名反射的脆弱耦合

"_instanceCounter" 的硬编码反射在内部重命名或类型调整时会导致与业务语义无关的测试失败。若短期仍保留反射，至少补充字段类型断言和失败信息，减少排障成本。

最小增强（当前文件内可做）

     private static void SetInstanceCounter(UiRouterBase router, int value)
     {
         var field = typeof(UiRouterBase).GetField("_instanceCounter", BindingFlags.Instance | BindingFlags.NonPublic);
-        Assert.That(field, Is.Not.Null);
+        Assert.That(field, Is.Not.Null, "UiRouterBase._instanceCounter 字段未找到，可能发生了内部重构。");
+        Assert.That(field!.FieldType, Is.EqualTo(typeof(int)), "_instanceCounter 字段类型已变化，请同步调整测试。");
 
-        field!.SetValue(router, value);
+        field.SetValue(router, value);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 173 - 179,
SetInstanceCounter uses hard-coded reflection on UiRouterBase's private field
"_instanceCounter", which is brittle; update the method to include explicit
assertions and messages and a field-type check before setting: locate the field
via typeof(UiRouterBase).GetField("_instanceCounter", BindingFlags.Instance |
BindingFlags.NonPublic), Assert.That(field, Is.Not.Null, "Expected private field
'_instanceCounter' on UiRouterBase not found"), Assert.That(field.FieldType,
Is.EqualTo(typeof(int)), "Expected '_instanceCounter' to be of type int"), then
call field.SetValue(router, value); this preserves the reflection approach but
gives clearer failure diagnostics and guards against type changes in
SetInstanceCounter/UiRouterBase.

GFramework.Game.Tests/UI/UiRouterInteractionTests.cs (1)

345-346: 建议使用 Allman 风格大括号包裹 if 分支

这里是非平凡行为（OnShow 内触发恢复），建议显式大括号以减少后续维护时误改风险，并与仓库统一风格保持一致。

♻️ 建议修改

-            if (ResumeFromShow)
-                OnResume();
+            if (ResumeFromShow)
+            {
+                OnResume();
+            }

As per coding guidelines: `Use Allman braces in C# code`.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@GFramework.Game.Tests/UI/UiRouterInteractionTests.cs` around lines 345 - 346,
Wrap the single-line conditional in Allman-style braces to make the non-trivial
behavior explicit: replace the current if (ResumeFromShow) OnResume(); with an
if block that opens and closes on their own lines and calls OnResume() inside
(preserving the ResumeFromShow and OnResume identifiers and their current
semantics), ensuring consistent C# Allman brace style with the rest of the
repository.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes